-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
External prod stylesheet #1161
External prod stylesheet #1161
Conversation
package.json
Outdated
@@ -12,7 +12,7 @@ | |||
"build-ui": "cd ui && yarn install && yarn build", | |||
"cypress-open": "cd proxy && yarn cypress-open", | |||
"cypress-run": "cd proxy && yarn cypress-run", | |||
"copy-build": "mkdir -p build && cp -R root/dist/* build/ && cp -R legacy/dist/assets build/", | |||
"copy-build": "mkdir -p build && cp -R root/dist/* build/ && cp -R legacy/dist/assets build/ && cp ui/dist/static/css/*.css.map build/assets/css/ui.css.map && cp ui/dist/static/css/*.css build/assets/css/ui.css", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like potentially undefined behaviour, which file is actually copied? It would be better to just use fixed names here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this to be to use the module name here e.g. ...css/main.*.css
. Is that better?
@@ -1,61 +1,4 @@ | |||
const webpack = require("webpack"); | |||
const path = require("path"); | |||
const { edit, remove, getPaths } = require("@rescripts/utilities"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @rescripts/utilities
dependency can be dropped, although you may find it useful if we need to reconfigure mini-css-extract-plugin
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Done
QA
You may have to use
SKIP_PREFLIGHT_CHECK=true make ui
like I did.Fixes
Fixes: #1157.